Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Server based allocator #27

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Server based allocator #27

wants to merge 8 commits into from

Conversation

vimageDE
Copy link
Collaborator

@vimageDE vimageDE commented Nov 7, 2024

Introduces a first example allocator contract that focuses on a server-based approach. While a backend would need to keep track of the allocated balances of sponsors, the contract will be used to manage signers, verify signatures of claims and handle attests for transfers. It can also consume nonces on theCompact.

@vimageDE vimageDE requested a review from 0age November 7, 2024 15:14
0age and others added 2 commits November 7, 2024 08:52
merge main into server-based-allocator branch
// keccak256("Attest(address,address,address,uint256,uint256)")
bytes4 private constant _ATTEST_SELECTOR = 0x1a808f91;

// keccak256("RegisterAttest(address signer,bytes32 attestHash,uint256 expiration,uint256 nonce)")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nit: Attest is used in various places here as though it's a noun when it's actually a verb. We should be using Attestation to refer to it, or using it as a verb, but RegisterAttest doesn't seem correct (basically it should either be just Attest or RegisterAttestation, or something else entirely)

// keccak256("RegisterAttest(address signer,bytes32 attestHash,uint256 expiration,uint256 nonce)")
bytes32 private constant _ATTEST_TYPE_HASH = 0xaf2dfd3fe08723f490d203be627da2725f4ad38681e455221da2fc1a633bbb18;

// keccak256("NonceConsumption(address signer,uint256[] nonces,bytes32[] attests)")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment here, "attests" doesn't seem right here

/// @inheritdoc IServerAllocator
function addSigner(address signer_) external onlyOwner {
if (_containsSigner(signer_)) {
return;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it make more sense to revert here with a SignerAlreadyAdded error?


address private immutable _COMPACT_CONTRACT;

mapping(address => uint256) private _signers;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe clearer to name this and/or include a comment to indicate that it actually maps signers to indices (plus one) in the array of signers?

uint256 nonceLength = attests_.length;
for (uint256 i = 0; i < nonceLength; ++i) {
bytes32 hashToConsume = attests_[i];
if (hashToConsume != bytes32(0)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when would a bytes32(0) value be supplied here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants